Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set status to 'not-busy' when server startup is complete #608

Merged
merged 2 commits into from
Sep 9, 2024
Merged

Set status to 'not-busy' when server startup is complete #608

merged 2 commits into from
Sep 9, 2024

Conversation

MichaReiser
Copy link
Member

Summary

Ensures that the extension calls updateStatus with busy=false when the server startup is complete.

This is now necessary after #600 correctly added an updateStatus call with busy=true before the server starts.

Fixes #605

Test Plan

I ran the extension and verified that the busy indicator no longer spins forever.

@MichaReiser MichaReiser added the bug Something isn't working label Sep 8, 2024
@dhruvmanila
Copy link
Member

I'm confused, where did #600 add the updateStatus call? I think that PR only changed the ordering such that the disposables are added only after a successful server start.

@MichaReiser
Copy link
Member Author

Hmm good call. I have to look into this again what the root cause is for it breaking

@dhruvmanila
Copy link
Member

The bug was that the onDidChangeState handler was being added after the server started so when the client updated the status to running, the handler wasn't present which would've changed the status. I've pushed a commit that changed the order back and added a helper function to dispose and remove them from the array and is being used when (a) the server failed to start or (b) when stopping the server.

@dhruvmanila dhruvmanila merged commit 69a99f6 into main Sep 9, 2024
6 checks passed
@dhruvmanila dhruvmanila deleted the micha/fix-server-status branch September 9, 2024 14:26
@dhruvmanila dhruvmanila mentioned this pull request Sep 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VSCode status bar display is weird in the latest (2024.44.0)
2 participants
@MichaReiser @dhruvmanila